[AlarmDecoder] Added support for AlarmDecoder LRR Protocol #5019
Conversation
Overview of LRR Support : http://www.alarmdecoder.com/wiki/index.php/LRR_Support Alarm Decoder LRR Protocol : http://www.alarmdecoder.com/wiki/index.php/Protocol#LRR Signed-off-by: Lucky Mallari <luckymallari@gmail.com>
Thanks @LuckyMallari! @berndpfrommer OK to merge or do you have changes to suggest? |
@watou There are no breaking changes, so should be OK to just merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- why is NUMTYPES initialized to 7? Shouldn't it be 6? Looks like a bug to me.
- there are also changes to a different binding "diyonxbee?" included in the commit. These should not be commingled with this commit. The commit should only refer to the alarmdecoder binding.
- there are a number of cosmetic changes (maybe it is space vs tabs, I can't see the difference). These changes should not be committed, as they obscure the true changes. If reformatting the code is necessary, that should be done in a separate commit that has no functional changes, only reformatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @berndpfrommer for your review! I agree with your first point. The minor reformats are a good side-effect of the formatter in the Eclipse IDE, so I think they are OK. And I don't see any changes to diyonxbee, so maybe that was removed with the second commit. In any case, @LuckyMallari, please adress the comment below and I believe it will be ready for merge, and thanks to both of you.
NUMTYPES(5); // total number of types | ||
LRR(4), // long range radio message | ||
INVALID(5), // invalid message | ||
NUMTYPES(7); // total number of types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @berndpfrommer's comment:
why is NUMTYPES initialized to 7? Shouldn't it be 6? Looks like a bug to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Perhaps a typo as I was trying a custom AlarmDecoder format.
Corrected NUMTYPES initialized value.
Thanks @LuckyMallari! Please update the wiki to include this enhancement, noting its first appearance in the 1.10.0 release. |
Thank you very much, and thanks for the heads-up as well, since I'm in the midst of converting the wiki pages to README.md files so they end up at docs.openhab.org. (See https://github.com/watou/openhab/tree/ded066c197e08d251346827e7e1ac77ef2413a5b/bundles/binding/org.openhab.binding.alarmdecoder) where I've tried to capture your new edits.) |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/added-support-for-alarmdecoder-lrr-protocol/20762/1 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/migrating-my-lua-scripts-to-xtend/38038/2 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/can-alarm-system-sensors-serve-double-duty/39352/7 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/alarmdecoder-help-ad2pi/42482/17 |
Overview of LRR Support : http://www.alarmdecoder.com/wiki/index.php/LRR_Support
Alarm Decoder LRR Protocol : http://www.alarmdecoder.com/wiki/index.php/Protocol#LRR
Sample usage:
Item MUST be in this format:
String sAlarmLRR "LLR Msg: [%s]" (gAlarmPanel) {alarmdecoder="LRR:00#text"}
ITEMS:
RULES: